Repository navigation
fix(app): ask for one name when alerting a new saved search - #3563
Conversation
Creating an alert from the search page showed two name fields: a "Saved search name" input in the modal header and the alert's own "Name" field, with no way to tell which named what. An unsaved search is now named once, inline in the header where the saved search page names it, and the alert inherits that name and the tags entered alongside it (the server already derives both when the create payload omits them). On a saved search the alert's field is labelled "Alert name" and shows the inherited name as a placeholder instead of pre-filling a copy of it.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 15ee1bf The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🟡 Tier 3 — StandardIntroduces new logic, modifies core functionality, or touches areas with non-trivial risk. Why this tier:
Review process: Full human review — logic, architecture, edge cases. Stats
|
Duplicate code analysisNo new duplicate code introduced by this PR. jscpd compared this branch against |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| </Tags> | ||
| </Group> | ||
| {nameError ? ( | ||
| <Text size="xs" c="red" data-testid="saved-search-name-error"> |
There was a problem hiding this comment.
🔵 minor — New validation text uses the raw palette color c="red" instead of the semantic Text variant
Use <Text size="xs" variant="danger" …>. The Repository conventions section 'Semantic component variants' says inline validation text should use variant="danger", not c="red".
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| data-testid="saved-search-name-input" | ||
| /> | ||
| <Tags allowCreate values={tags} onChange={onTagsChange}> | ||
| <Button |
There was a problem hiding this comment.
🔵 minor — Tags button has no accessible name: screen readers announce only the count ("0")
Add something like aria-label={Saved search tags (${tags.length})} to the Button. Its only content is the decorative IconTags and a number, so nothing tells a screen-reader user what the control does.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
PR Review
5 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 5 minor 2 posted as inline comment(s) on the changed lines. 3 listed below. Findings outside the changed lines3 minor
Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides. |
Deep Review✅ No critical issues found. The diff is a clean, well-tested UI refactor: the two schemas it relies on are 🟡 P2 — recommended
🔵 P3 nitpicks (6)
Reviewers (6): ce-correctness-reviewer, ce-kieran-typescript-reviewer, ce-julik-frontend-races-reviewer, ce-testing-reviewer, ce-maintainability-reviewer, ce-adversarial-reviewer. Testing gaps:
No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond |
E2E Test Results✅ All tests passed • 463 passed • 1 skipped • 1822s
Tests ran across 4 shards in parallel. |
Summary
Creating an alert from the search page asked for two names and never said which was which.
On an unsaved search the modal showed a bordered "Saved search name" input in its header and the alert's own "Name" field, whose placeholder read "Defaults to the saved search or dashboard tile name" — a name field explaining itself in terms of a name field four pixels above it. On a saved search the header already read "Alerts for Checkout errors" and the alert's Name field was pre-filled with "Checkout errors" again, which reads as a duplicate rather than an override.
This is a prototype of the simplification: one name per screen.
InlineNameInput("Untitled search") the saved search page uses for the same object. The alert's own name and tags row is gone — the create payload omitsdisplayName/tags, which the server already derives from the saved search it is attached to, so the alert comes out named after the search either way. The tags button next to the name now tags the search (previously it tagged only the alert and the new search was silently saved untagged).No API change. Removing the client-side prefill relies on
deriveAlertDisplayFields, which already clamps the name and tags to the alert caps — the same clamping the client was doing by hand withclampAlertDisplayName/clampAlertTags.Screenshots or video
Both captured on
yarn dev, same viewport and theme.Alerting an unsaved search — two name fields before, one after:
Alerting a saved search — pre-filled duplicate before, inherited placeholder after:
Unnamed search on the first submit:
The alert created this way lands with the search's name and tags:
How to reproduce
Starting point:
yarn dev(the Alerts button is hidden in local mode, so the demo server and the Vercel preview can't reach this modal). Register, then open Search and pick any logs source.On
main:Checkout errors, then click Alerts again: the header says "Alerts for Checkout errors" and the Name field repeatsCheckout errors.On this branch:
Payment failures, add the tagpaymentswith the button beside the name, pick a webhook, and save. You land on the new saved search, and Alerts lists an alert namedPayment failurestaggedpayments.Payment failures, with tags reading "Inherited".How to test on Vercel preview
N/A — the Alerts button is not rendered in local mode (
IS_LOCAL_MODE), so the preview build cannot reach this modal.References
Fixes HDX-5917